Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

toml: move from toml to tomllib + tomli_w #395

Merged
merged 1 commit into from
Aug 5, 2023

Conversation

parona-source
Copy link
Contributor

@parona-source parona-source commented Aug 3, 2023

  • toml is getting removed from distributions due to being generally unmaintained and not supporting the latest TOML standard.
  • For parsing use tomllib which is included in python3.11 and tomli for earlier interpreter versions.
  • For writing use tomli-w. tomli-w.dump() expects bytes-like objects so work around it to keep compatibility and minimize changes elsewhere.

https://fedoraproject.org/wiki/Changes/DeprecatePythonToml
https://bugs.gentoo.org/878651

Ran the full test-ng test suite in Ubuntu container with python3.8 as its failing on my host machine for unrelated reasons in current HEAD. Haven't looked further on that. Pytest and toml.sh pass just fine.

Arch appears to be on python3.11 so it should just need tomli-w.

@parona-source
Copy link
Contributor Author

************* Module yaml_to_toml
scripts/yaml_to_toml.py:55:4: C0103: Constant name "out" doesn't conform to UPPER_CASE naming style (invalid-name)

I'm unsure how my change triggered this.

@deadc0de6
Copy link
Owner

Oh very cool, thanks a lot for this!

I'm also surprised by the pylint error since I have other actions using the same pylint/python version that did not raise that linting error. It seems it has been raised already, see pylint-dev/pylint#3132.

I added pylint comments in 84bdd80 what should hopefully bypass those linting errors. Could you rebase?

* toml is getting removed from distributions due to being generally
  unmaintained and not support the lastest TOML standard.
* For parsing use tomllib which is included in python3.11 and tomli for
  earlier interpeter versions.
* For writing use tomli-w. tomli-w.dump() expects bytes-like objects so
  work around it to keep compatability and minimize changes elsewhere.

Signed-off-by: Alfred Wingate <[email protected]>
@parona-source
Copy link
Contributor Author

parona-source commented Aug 3, 2023

ask@bigglane /home/ask/sources/test
 $ cat t.py 
'''
cool
'''
import tomli_w

if __name__ == '__main__':
    out = tomli_w.dumps("")
    print(out)
ask@bigglane /home/ask/sources/test
 $ pylint t.py
************* Module t
t.py:7:4: C0103: Constant name "out" doesn't conform to UPPER_CASE naming style (invalid-name)

------------------------------------------------------------------
Your code has been rated at 7.50/10 (previous run: 7.50/10, +0.00)

ask@bigglane /home/ask/sources/test
 $ cat t1.py 
'''
cool
'''
import toml

if __name__ == '__main__':
    out = toml.dumps("test")
    print(out)
ask@bigglane /home/ask/sources/test
 $ pylint t1.py

-------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 8.75/10, +1.25)

Something with tomli_w is cursed :P

@coveralls
Copy link

coveralls commented Aug 3, 2023

Coverage Status

coverage: 91.052% (+0.003%) from 91.049% when pulling 91daad5 on parona-source:toml into 84bdd80 on deadc0de6:master.

@parona-source
Copy link
Contributor Author

parona-source commented Aug 4, 2023

Ran the full test-ng test suite in Ubuntu container with python3.8 as its failing on my host machine for unrelated reasons in current HEAD.

On an unrelated note ive gotten to minimize the issue with tests-ng/import-with-trans.s to the gnupg version. Something with behaviour changed between 2.2.41 and 2.4.2 which causes it to fail. Haven't further time on this. But if you update the ci container at some point in the future, it may then become an issue.

@deadc0de6
Copy link
Owner

Thanks. I have opened an issue at pylint with your example: pylint-dev/pylint#8930.

@deadc0de6
Copy link
Owner

Ran the full test-ng test suite in Ubuntu container with python3.8 as its failing on my host machine for unrelated reasons in current HEAD.

On an unrelated note ive gotten to minimize the issue with tests-ng/import-with-trans.s to the gnupg version. Something with behaviour changed between 2.2.41 and 2.4.2 which causes it to fail. Haven't further time on this. But if you update the ci container at some point in the future, it may then become an issue.

Thanks, good point, I'll move away from gpg to avoid any bad surprise

@deadc0de6 deadc0de6 merged commit 56d5a86 into deadc0de6:master Aug 5, 2023
@deadc0de6
Copy link
Owner

@parona-source thanks again for your help on this! I'll release a new version ASAP.

@parona-source parona-source deleted the toml branch August 6, 2023 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants